fix(transport): AiohttpTransport.aclose() should not close shared ClientSession#21287
fix(transport): AiohttpTransport.aclose() should not close shared ClientSession#21287ArivunidhiA wants to merge 2 commits intoBerriAI:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR adds an
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| litellm/llms/custom_httpx/aiohttp_transport.py | Adds _owns_session flag to prevent closing shared ClientSessions on aclose(). All 5 session-creation code paths correctly set the flag. Minor gap: _get_valid_client_session may still close a shared session during event loop mismatch. |
| tests/test_litellm/llms/custom_httpx/test_aiohttp_transport.py | Two well-targeted tests verify the core fix: shared sessions are not closed, factory-created sessions are. No real network calls. Minor whitespace cleanup in existing tests. |
Flowchart
flowchart TD
A[LiteLLMAiohttpTransport.__init__] --> B{client is ClientSession?}
B -- Yes --> C["_owns_session = False\n(shared session)"]
B -- No/Factory --> D["_owns_session = True\n(factory/will create)"]
C --> E[aclose called]
D --> E
E --> F{_owns_session?}
F -- False --> G["Skip close\n(shared session safe)"]
F -- True --> H{client is ClientSession?}
H -- Yes --> I[Close session]
H -- No --> G
D --> J[_get_valid_client_session]
J --> K{Session valid?}
K -- No --> L["Create new session\n_owns_session = True"]
K -- Yes --> M[Return existing session]
L --> N[handle_async_request retry]
N --> O["Create new session\n_owns_session = True"]
Last reviewed commit: 75dddbd
Additional Comments (1)
When a shared Consider guarding this close call with This keeps the behavior consistent: if we don't own the session, we shouldn't close it anywhere — just replace our reference to it. |
Address Greptile review: _get_valid_client_session could still close a shared session during event loop mismatch. Now checks _owns_session before attempting to close the old session.
8d5b403 to
e748da3
Compare
|
The 6 failing "LiteLLM Unit Tests (Matrix)" checks (integrations, llms, proxy-guardrails, proxy-unit-a, proxy-unit-b, root) are pre-existing failures that affect all fork PRs due to missing repo secrets. These same tests fail on unrelated docs-only PRs (e.g., #21283, #21282). All checks that can run on fork PRs pass successfully. |
Fixes #21116
When a shared
ClientSessionis passed toLiteLLMAiohttpTransport,aclose()unconditionally closes it, breaking other clients still using the same session.AiohttpHandleralready handles this correctly with_owns_session(added in #19190), butAiohttpTransportwas missed.Fix: Added
_owns_sessionflag toAiohttpTransport:Falsewhen a pre-existingClientSessionis passed in (shared — don't close)Truewhen a factory is passed or when the transport creates a new session itselfaclose()now only closes the session if the transport owns itAll 5 code paths where new sessions are created (
_get_valid_client_session, retry inhandle_async_request) set_owns_session = True.Changed files:
litellm/llms/custom_httpx/aiohttp_transport.pytests/test_litellm/llms/custom_httpx/test_aiohttp_transport.py(2 new tests)All 14 transport tests pass. ruff clean.